Skip to content

Comments

fix(snapshot): changed insert to upsert when concurrent identical child workflows are running#3259

Merged
waleedlatif1 merged 2 commits intostagingfrom
fix/child
Feb 19, 2026
Merged

fix(snapshot): changed insert to upsert when concurrent identical child workflows are running#3259
waleedlatif1 merged 2 commits intostagingfrom
fix/child

Conversation

@waleedlatif1
Copy link
Collaborator

@waleedlatif1 waleedlatif1 commented Feb 19, 2026

Summary

  • changed insert to upsert when concurrent identical child workflows are running to avoid insertion conflicts

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Feb 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Feb 19, 2026 9:13pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 19, 2026

Greptile Summary

Fixes race condition in concurrent snapshot creation by replacing SELECT-then-INSERT pattern with atomic upsert operation.

Key changes:

  • Removed getSnapshotByHash query + manual UPDATE in favor of INSERT ... ON CONFLICT DO UPDATE
  • Added logic to detect if snapshot is new by comparing returned ID with generated ID
  • Enhanced logging to differentiate between new and reused snapshots
  • Added comprehensive tests covering concurrent execution scenarios
  • Simplified code by eliminating separate SELECT query

Confidence Score: 5/5

  • Safe to merge - solid race condition fix with comprehensive tests
  • Clean implementation that replaces problematic check-then-act pattern with atomic upsert. Well-tested with concurrent execution scenarios.
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/lib/logs/execution/snapshot/service.ts Replaced SELECT-then-INSERT pattern with atomic upsert to prevent race conditions
apps/sim/lib/logs/execution/snapshot/service.test.ts Added comprehensive tests for upsert behavior including concurrency scenarios

Sequence Diagram

sequenceDiagram
    participant C1 as Concurrent Request 1
    participant C2 as Concurrent Request 2
    participant DB as PostgreSQL Database
    
    Note over C1,C2: Both compute same stateHash for identical workflows
    
    C1->>DB: INSERT with UUID-1<br/>ON CONFLICT (workflowId, stateHash)<br/>DO UPDATE SET stateData
    C2->>DB: INSERT with UUID-2<br/>ON CONFLICT (workflowId, stateHash)<br/>DO UPDATE SET stateData
    
    Note over DB: Unique index on (workflowId, stateHash)<br/>ensures atomicity
    
    alt First request wins
        DB-->>C1: Returns snapshot with UUID-1
        DB-->>C2: Returns existing snapshot with UUID-1
        Note over C1: isNew = true (UUID matches)
        Note over C2: isNew = false (UUID differs)
    else Second request wins
        DB-->>C2: Returns snapshot with UUID-2
        DB-->>C1: Returns existing snapshot with UUID-2
        Note over C2: isNew = true (UUID matches)
        Note over C1: isNew = false (UUID differs)
    end
Loading

Last reviewed commit: fd967b2

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator Author

@cursor agent

@cursor
Copy link

cursor bot commented Feb 19, 2026

Taking a look!

Open in Cursor Open in Web

@waleedlatif1 waleedlatif1 merged commit 2b5e436 into staging Feb 19, 2026
11 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/child branch February 19, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant